Fix: correctly pluralize 'target'/'targets' in finished message #16132
Conversation
rustbot
added
A-build-execution
| ); | ||
| let target_count = self.counts.len(); | ||
| let message = format!( | ||
| "{profile_link}`{profile_name}` profile [{opt_type}]{profile_link:#} target(s) in {time_elapsed}", |
There was a problem hiding this comment.
This message is emitted in every build command, and changing this failed 960 tests. To fix them we then need to update snapshots in those 960 tests. I am not sure if it is worthy.
| let profile_link = build_runner.bcx.gctx.shell().err_hyperlink( | ||
| "https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles", | ||
| ); | ||
| let target_count = self.counts.len(); |
There was a problem hiding this comment.
This seems wrong as it counted every package in the dependency graph, not just selected local packages.
This change ensures that the finished message correctly uses "target" or "targets"
based on the number of targets built, improving clarity in multi-target builds.
However, maybe the question should be asked like: What do we really want to count here? What does target(s) mean here really?
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
Going to close this as it has some outstanding question need to be discussed first as an issue:
Thanks for being interested in this anyway! |
4 participants
This change ensures that the finished message correctly uses "target" or "targets"
based on the number of targets built, improving clarity in multi-target builds.